Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Common feed format #5

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

porglezomp
Copy link

This is a first attempt at defining a common structure for both Atom and RSS feeds. It's not complete yet, and it doesn't provide the best round trip guarantees right now. Feedback is very much appreciated.

In addition to looking at the docs for both atom_syndication and rss, the information at RSS and Atom Compared has been extremely helpful while implementing this.

@porglezomp
Copy link
Author

Do we want to expose only the common format as the public API?

@tomshen
Copy link
Collaborator

tomshen commented Jun 13, 2016

Thanks for the contribution!

We should only expose the common format as the public API. The purpose of this library is to define a common format so that you can use the same code for working on both Atom and RSS feed, but I just never got around to finishing it. Exposing the Atom/RSS library-specific formats defeats that purpose.

I'll try to take a look at the code sometime this week.

Cargo.toml Outdated
@@ -1,13 +1,14 @@
[package]
name = "syndication"
version = "0.1.1"
authors = ["Tom Shen <[email protected]>"]
version = "0.1.2"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should rebase your branch, since the library is already on version "0.2.0." This change should bump the version to "0.3.0," since it breaks the original API.

@porglezomp
Copy link
Author

porglezomp commented Jun 15, 2016

Right now I'm considering doing

use atom_syndication as atom;

since it gives better symmetry with rss, and it's less annoying since I have to disambiguate the types a lot in some of the modules. Is this okay, or should I avoid this?

@tomshen
Copy link
Collaborator

tomshen commented Jun 15, 2016

That looks fine to me.

The public API shouldn't expose any types from rss or atom_syndication,
so this adds all the necessary types.
Add Guid type since the RSS Guid is more detailed than the Atom one.
Add the Image and TextInput types to support the change.
EntryData and FeedData need an explicitly implementation of Debug
because the atom types don't support it, and we probably don't want to
show the contained feeds anyway.
@porglezomp
Copy link
Author

There aren't very good tests for this library, are there. Trying to write some, I guess I can see why. Any ideas for what I should do on the testing front?

@porglezomp
Copy link
Author

Should I put myself as an author?

@tomshen
Copy link
Collaborator

tomshen commented Jun 15, 2016

There aren't very good tests for this library, are there. Trying to write some, I guess I can see why. Any ideas for what I should do on the testing front?

You could use tests from other open source libraries. e.g. https://github.com/kurtmckee/feedparser/tree/develop/tests

Should I put myself as an author?

Definitely, especially since after this PR merges, most of the codebase will be your work.

Honestly, I haven't looked at Rust stuff in a really long time. So if you want, I can just transfer ownership of the repo/crate to you.

@porglezomp
Copy link
Author

I just finished up classes for the year, and I accidentally broke my main RSS reader, so I figure now's a good time to come back to this. I'm going to try to use this to build a small feed reader in order to figure out if the API has any glaring holes.

@nc7s
Copy link

nc7s commented Feb 2, 2024

Hi, is there still motivation for this? I'm currently writing an application that requires unified RSS & Atom parsing, this would be a great help. Willing to pick it up if there's no time for this on your side.

@porglezomp
Copy link
Author

My job means I'm currently not able to easily make open source contributions, I would be happy with someone else picking up the work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants